Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

executor: support GROUP_CONCAT(ORDER BY) (#16591) #16990

Merged
merged 3 commits into from
May 13, 2020
Merged

executor: support GROUP_CONCAT(ORDER BY) (#16591) #16990

merged 3 commits into from
May 13, 2020

Conversation

sre-bot
Copy link
Contributor

@sre-bot sre-bot commented May 6, 2020

cherry-pick #16591 to release-4.0

What problem does this PR solve?

Issue Number: close #6838

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

Executor part:

  1. Add a heap in GROUP_CONCAT function implementation.
  2. Build hash agg in unparalleled if group_concat has ORDER BY clause.

Planner part:

  1. Build and resolve arguments of ORDER BY clause.
  2. Add explain info with ORDER BY clause.
  3. Handle arguments of ORDER BY clause in ResolveIndices, PruneColumns and InjectProjBelowAgg
  4. Do not push down to coprocessor if group_concat has ORDER BY clause.
  5. Do not push down to Union and Join if group_concat has ORDER BY clause.

How it Works:

mysql> desc select 1, 2, 3, 4, 5 , group_concat(name, id ORDER BY 2 desc, name SEPARATOR '++') from test;
+------------------------------+---------+-----------+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| id                           | estRows | task      | access object | operator info                                                                                                                                        |
+------------------------------+---------+-----------+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
| Projection_4                 | 1.00    | root      |               | 1->Column#5, 2->Column#6, 3->Column#7, 4->Column#8, 5->Column#9, Column#4                                                                            |
| └─StreamAgg_8                | 1.00    | root      |               | funcs:group_concat(Column#15, Column#16 order by Column#17 desc, Column#18 asc separator "++")->Column#4                                             |
|   └─Projection_18            | 6.00    | root      |               | cast(test.test.name, var_string(20))->Column#15, cast(test.test.id, var_string(20))->Column#16, test.test.name->Column#17, test.test.name->Column#18 |
|     └─TableReader_15         | 6.00    | root      |               | data:TableFullScan_14                                                                                                                                |
|       └─TableFullScan_14     | 6.00    | cop[tikv] | table:test    | keep order:false, stats:pseudo                                                                                                                       |
+------------------------------+---------+-----------+---------------+------------------------------------------------------------------------------------------------------------------------------------------------------+
5 rows in set (0.00 sec)

mysql> select 1, 2, 3, 4, 5 , group_concat(name, id ORDER BY 2 desc, name SEPARATOR '++') from test;
+---+---+---+---+---+-------------------------------------------------------------+
| 1 | 2 | 3 | 4 | 5 | group_concat(name, id ORDER BY 2 desc, name SEPARATOR '++') |
+---+---+---+---+---+-------------------------------------------------------------+
| 1 | 2 | 3 | 4 | 5 | 5003++2003++301++201++202++101                              |
+---+---+---+---+---+-------------------------------------------------------------+
1 row in set (0.00 sec)

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Side effects

  • Performance regression
    • Consumes more MEM

Release note

  • support ORDER BY clause in GROUP_CONCAT

@sre-bot sre-bot requested review from a team as code owners May 6, 2020 10:52
@sre-bot
Copy link
Contributor Author

sre-bot commented May 6, 2020

/run-all-tests

@sre-bot sre-bot mentioned this pull request May 6, 2020
Merged
@sre-bot sre-bot added sig/execution SIG execution component/expression sig/planner SIG: Planner release-note Denotes a PR that will be considered when it comes time to generate release notes. status/PTAL type/4.0-cherry-pick type/new-feature labels May 6, 2020
@ghost ghost requested a review from wshwsh12 May 6, 2020 10:52
@ghost ghost requested review from lzmhhh123 and removed request for zz-jason, eurekaka, qw4990, winoros, XuHuaiyu, francis0407 and lzmhhh123 May 6, 2020 10:52
@sre-bot sre-bot added this to the v4.0.0-rc.1 milestone May 6, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 8, 2020

@wshwsh12, PTAL.

@bb7133 bb7133 modified the milestones: v4.0.0-rc.1, 4.0.0-rc.2 May 9, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 10, 2020

@wshwsh12, PTAL.

wshwsh12
wshwsh12 reviewed May 11, 2020
View reviewed changes
Copy link
Contributor

@wshwsh12 wshwsh12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zz-jason
zz-jason approved these changes May 11, 2020
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. and removed status/PTAL labels May 11, 2020
@sre-bot
Copy link
Contributor Author

sre-bot commented May 11, 2020

/run-all-tests

@sre-bot
Copy link
Contributor Author

sre-bot commented May 11, 2020

@sre-bot merge failed.

@sre-bot
Copy link
Contributor Author

sre-bot commented May 13, 2020

@wshwsh12, @zz-jason, PTAL.

@SunRunAway
Copy link
Contributor

/merge

@sre-bot
Copy link
Contributor Author

sre-bot commented May 13, 2020

/run-all-tests

@SunRunAway
Copy link
Contributor

@sre-bot merge failed.

#15531

@sre-bot sre-bot merged commit ca87109 into pingcap:release-4.0 May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wshwsh12 wshwsh12 wshwsh12 left review comments

@zz-jason zz-jason zz-jason approved these changes

Assignees

@SunRunAway SunRunAway

Labels
component/expression release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/new-feature type/4.0-cherry-pick
Projects
None yet
Milestone
4.0.0-rc.2
Development

Successfully merging this pull request may close these issues.
6 participants
@sre-bot @SunRunAway @zz-jason @wshwsh12 @bb7133 @ngaut